ci: wire up extension-update-test (0.1.0 -> stable) - #19
Open
jnasbyupgrade wants to merge 3 commits into
Open
Conversation
…ting Adds the committed install->guard->update->assert->run-suite script (bin/test_existing, modeled on cat_tools's bin/test_existing) plus a generic per-extension structural-diff tool (bin/structural_diff[.sql], copied near-verbatim from cat_tools -- it's already written generically off pg_depend's deptype='e' membership edge) and a new CI job that exercises the 0.1.0->stable update path end to end: install 0.1.0, plant + prove the dependency guard, ALTER EXTENSION UPDATE, structurally compare against a fresh "stable" install, then run the full suite in existing mode. No binary pg_upgrade job is added: object_reference has no view/function that SELECTs * over a system catalog in either its current or 0.1.0 install script (checked directly), so the cross-PostgreSQL-major risk that job protects against is low here. Left as noted future work rather than built preemptively -- see the ci.yml "Test strategy" comment and the PR description. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds the binary pg_upgrade coverage this PR previously deferred: ONE
cluster starting at the floor PostgreSQL major with 0.1.0 installed,
updated straight to the current version, then climbing every later
supported major in sequence via a real binary pg_upgrade, running the
full suite (existing mode) and re-proving the dependency guard after
every step.
Per review of advanced-extension-testing.md's guidance: "unlikely to
catch anything today" (no SELECT * over a system catalog found in
object_reference's views/functions) is a weaker, non-self-correcting
reason to skip a cheap job than a genuine cost argument -- an
extension can grow catalog-touching code later without anyone
revisiting a stale "skip, it's simple" decision.
- bin/test_existing: add `prepare-old DB [INSTALL_VERSION]`, refactored
out of update-scenario's existing create+guard logic. Simpler than
cat_tools's own (no BRIDGE_TO parameter) since 0.1.0 has no identified
pg_upgrade-unsafe construct to bridge away from.
- .github/workflows/ci.yml:
- `changes` job now also derives `climb_pg`, an ascending PG-major
list from the same NEWEST/CURRENT_FLOOR constants the `test` job's
matrix already uses -- no separate LEGACY_FLOOR, since 0.1.0
installs cleanly across the whole supported range.
- New `pg-upgrade-stepwise` job, gated behind lint+test like
extension-update-test.
- `all-checks-passed` needs updated to include it.
- Top-of-file "Test strategy" comment updated: pg_upgrade coverage is
no longer deferred.
make lint clean.
Move the _sentry_mv / binary-pg_upgrade root-cause analysis into a code comment on the job itself, so it's visible to anyone reading ci.yml directly, not just in the PR description.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #18 (link 3/3 of the update+upgrade testing effort -- completes
checklist item 7 of
advanced-extension-testing.md§10: CI wiring for theextension-update test, on top of #18's local
test/install/load.sqlthree-mode machinery).
What this adds
bin/test_existing-- the committed, parameterizedinstall→guard→update→assert→run-suite script (subcommands:
plant-guard,update,prepare-old,run-suite,update-scenario,diff-fresh), modeled onPostgres-Extensions/cat_tools'sbin/test_existing.prepare-oldis simpler than cat_tools's own (noBRIDGE_TOparameter -- see the script's header for why);update-check/
update-check-versionaren't needed here at all (also explained in theheader).
bin/structural_diff/bin/structural_diff.sql-- a genericper-extension structural comparison tool, copied near-verbatim from
cat_tools (already written generically, no cat_tools-specific object
names). Wired into
update-scenarioasassert_matches_fresh.extension-update-testCI job: installs 0.1.0 (+ its since-droppedcount_nullsdependency), plants + proves the dependency guard,ALTER EXTENSION UPDATEs to the current version, structurally compares againsta fresh
stableinstall, then runs the full suite against the realupdated database in
existingmode. Runs on a single PostgreSQL major(the newest supported) -- 0.1.0 has no identified PG-version floor, so
crossing the full matrix would add job count with no added coverage.
pg-upgrade-stepwiseCI job: ONE cluster starting at the floorPostgreSQL major with 0.1.0 installed, updated straight to the current
version, then climbing every later supported major in sequence
(12→13→...→18) via a real binary
pg_upgradeper step, running the fullsuite and re-proving the dependency guard after every step. The
changesjob now also derivesclimb_pg(ascending PG-major list) fromthe same
NEWEST/CURRENT_FLOORconstants thetestjob's matrixalready uses.
all-checks-passed'sneeds:list updated for both new jobs; top-of-file"Test strategy" comment updated to match.
A real per-repo adaptation from the cat_tools reference
bin/test_existing's dynamic version assertion derives the currentversion from
make -s print-EXTENSION_object_reference_VERSION, notprint-PGXNVERSION(which cat_tools's own script uses). object_referencehas already adopted the
stablepseudo-version scheme (#18):PGXNVERSIONis frozen at0.1.0(the last real numbered PGXN release,used for git tagging/dist) and no longer tracks the current build's
default_versiononcestableis in play -- cat_tools'sPGXNVERSIONand
default_versionstill happen to match, so this divergence wasn'tvisible there. Confirmed directly:
make -s print-PGXNVERSION→0.1.0,make -s print-EXTENSION_object_reference_VERSION→stable(the correctone to assert against).
Known issue:
pg-upgrade-stepwisecurrently fails, not a CI-wiring bugThe job's first real binary
pg_upgradestep already exercises schemaanalyze/freeze,
pg_xact/pg_multixactcopy, WAL resets, and the newcluster's schema restore -- then fails restoring
_sentry_mv(
pg_class heap OID value not set when in binary upgrade mode, see thejob's own comment in
ci.ymlfor the full root-cause analysis: amaterialized view marked
pg_extension_config_dump()cannot survivebinary
pg_upgrade, confirmed as a general PostgreSQL limitation, notanything object_reference-specific). Fixing
_sentry_mvneeds amaintainer decision (it's load-bearing for object_reference's
restore-repair contract), so I haven't attempted one here -- this PR's CI
will stay red on
pg-upgrade-stepwise/all-checks-passeduntil that'sresolved separately. Everything else (
lint, thetestmatrix,extension-update-test) is green; CI run:https://github.com/Postgres-Extensions/object_reference/actions/runs/31049876539.
Verification
make lintclean.update-scenarioflow locally against a real PostgreSQL 17cluster before pushing: 0.1.0 install → guard planted + proven → update
to
stable→ structural diff clean → full suite green inexistingmode (7/7 pgTAP files passing).
RETURNvsEXITtraps undererrexit): deliberately corrupted acomment on a real object_reference table post-update to force a genuine
structural divergence, then ran
diff-freshdirectly. Confirmed (a) thescript exited non-zero (
FAIL: structural diff ..., exit code 1), and(b) the scratch
*__fresh_refreference database was still dropped bythe
EXITtrap despite the failure not being the trap-registeringsubshell's last command. Also confirmed via
trap -p EXITthat thescratch database name is baked into the trap as a literal at
registration time, not re-read from the variable when the trap fires.
_sentry_mvroot cause against a bare,non-extension materialized view in a scratch cluster to confirm it's
general, not object_reference-specific (see the
ci.ymlcomment).Test plan
make lintupdate-scenariodry run against a real clusterlint/testmatrix /extension-update-testgreenpg-upgrade-stepwise/all-checks-passedgreen -- blocked on the_sentry_mvissue above, needs a maintainer decision